Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add device_info_app_build_configuration property to all events #182

Merged
merged 3 commits into from
Jun 25, 2021

Conversation

koke
Copy link
Member

@koke koke commented Jun 16, 2021

Adds a new device_info_app_build_configuration property to all events that can be:

  • Debug: for local developer builds
  • Alpha: for Release-alpha builds, I believe we use this for Installable Builds on PRs. This requires the app integrating the library to define an ALPHA preprocessor macro.
  • Beta: for Release builds uploaded to TestFlight
  • Production: for Release builds uploaded to the App Store

Alpha releases can be configured with the following in the app's Podfile:

  # Flag Alpha builds for Tracks
  # ============================
  installer.pods_project.targets.each do |target|
    next unless target.name == "Automattic-Tracks-iOS"
    target.build_configurations.each do |config|
      next unless config.name == "Release-Alpha"
      config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] ||= ['$(inherited)', 'ALPHA=1']
    end
  end

I've only implemented this for Woo in woocommerce/woocommerce-ios#4468, so I'm not sure if integrating this in WordPress will require any changes for the WP Internal builds.

Part of woocommerce/woocommerce-ios#4432

To test

  1. Check out Include build configuration details in all tracks events woocommerce/woocommerce-ios#4468 and run the Woo app.
  2. You can set a breakpoint in - [TracksServiceRemote sendBatchOfEvents:withSharedProperties:completionHandler:] and ensure that the properties argument contains Debug for device_info_app_build_configuration.
  3. Check the Tracks Live View after a few minutes, and verify that the events coming from the app include the new property.

I haven't been able to test anything other than Debug builds yet 😞

@koke
Copy link
Member Author

koke commented Jun 23, 2021

@Automattic/platform-9 I could use some help testing this in configurations other than Debug. I'm not even sure if we can test the TestFlight one without actually pushing a new update, so maybe something to watch out for in the next release cycle

@mokagio
Copy link
Contributor

mokagio commented Jun 24, 2021

Maybe we should setup distribution for the Tracks-Demo app that's part of the project via App Center?

Copy link
Member

@ctarda ctarda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the testing instructions and I can see device_info_app_build_configuration in properties.

I tested only with the WCiOS app though. I think it would not hurt to test with WPiOS as well, sometime net week perhaps?

:shipit:

@koke
Copy link
Member Author

koke commented Jun 24, 2021

I think it would not hurt to test with WPiOS as well, sometime net week perhaps?

It is not integrated on WPiOS yet, but I'll give it a try next week

@koke koke force-pushed the add/build-configuration branch from 50d8c55 to 7b4abbc Compare June 25, 2021 06:23
@koke koke merged commit 6f21873 into develop Jun 25, 2021
@koke koke deleted the add/build-configuration branch June 25, 2021 06:34
@koke
Copy link
Member Author

koke commented Jun 30, 2021

For reference, tested this on a TestFlight release and it's showing the correct value 😌

Screen Shot 2021-06-30 at 11 08 57

@astralbodies
Copy link
Contributor

Thanks again for doing this, @koke!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants